Skip to content

Fix phpstan/phpstan#12665: should return array{a: string, b: int, c: int} but returns non-empty-array<'a'|'b'|'c', int|string>#5193

Merged
ondrejmirtes merged 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-qaxqf38
Mar 14, 2026
Merged

Fix phpstan/phpstan#12665: should return array{a: string, b: int, c: int} but returns non-empty-array<'a'|'b'|'c', int|string>#5193
ondrejmirtes merged 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-qaxqf38

Conversation

@phpstan-bot
Copy link
Collaborator

@phpstan-bot phpstan-bot commented Mar 10, 2026

Summary

When building an array in a foreach loop by assigning values at constant string keys from a union type, PHPStan would degrade the entire array type to a general non-empty-array<key, value> instead of preserving the constant array shape. This fix preserves the shape by adding unmatched keys as optional entries.

Before: non-empty-array<'a'|'b'|'c', int|string>
After: array{a: string, b?: int, c?: int}

Changes

  • Modified src/Type/Constant/ConstantArrayTypeBuilder.php: When setOffsetValueType receives a union of constant scalars where none match existing keys and the builder already has keys, the unmatched scalars are added as optional entries instead of degrading to a general array type
  • Added tests/PHPStan/Analyser/nsrt/bug-12665.php: Regression test reproducing the reported issue
  • Added tests/PHPStan/Analyser/nsrt/set-constant-union-offset-on-constant-array.php: Test for non-loop union offset setting
  • Updated tests/PHPStan/Analyser/nsrt/array-fill-keys.php: Two assertions updated to reflect improved type precision from the fix

Root cause

In ConstantArrayTypeBuilder::setOffsetValueType(), when the offset is a union of constant scalars (e.g., 'b'|'c'), the code extracts the individual scalar types and tries to match each one against existing keys. If ALL scalars match, it updates the value types. But if ANY scalar doesn't match, the entire matching was discarded and the code fell through to a degradation path that set degradeToGeneralArray = true, losing the constant array shape entirely.

The fix adds a new code path: when no scalars match existing keys (completely new keys being added) and the builder already has existing keys, the unmatched scalars are added as new optional entries. This preserves the constant array shape while correctly marking the new keys as optional (since only one of the union members is set in any given execution).

The condition count($this->keyTypes) > 0 ensures we only apply this behavior when extending an existing array, not when constructing a new one from scratch (which avoids regressions in array function return type extensions).

Test

The regression test bug-12665.php reproduces the exact scenario from the issue:

$array = ['a' => $s];
foreach (['b', 'c'] as $letter) {
    $array[$letter] = $i;
}
assertType('array{a: string, b?: int, c?: int}', $array);

Without the fix, PHPStan infers non-empty-array<'a'|'b'|'c', int|string>. With the fix, it correctly infers array{a: string, b?: int, c?: int}, preserving the constant array shape with the new keys marked as optional.

Fixes phpstan/phpstan#12665

Closes phpstan/phpstan#14080
Closes phpstan/phpstan#8774
Closes phpstan/phpstan#9907
Closes phpstan/phpstan#11006

- Modified ConstantArrayTypeBuilder::setOffsetValueType() to add unmatched
  scalar keys as optional entries instead of degrading to a general array,
  when none of the union members match existing keys and the array is non-empty
- New regression test in tests/PHPStan/Analyser/nsrt/bug-12665.php
- New test for non-loop union offset setting in nsrt/set-constant-union-offset-on-constant-array.php
- Updated array-fill-keys test expectations to reflect improved type precision

Closes phpstan/phpstan#12665
Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example

class Broken
{
	/** @return array{a: string, b: int, c: int} */
	public function break(string $s, int $i): array
	{
		$array = ['a' => $s];
		foreach (['b', 'c'] as $letter) {
			$array[$letter] = $i;
		}
		assertType('array{a: string, b?: int, c?: int}', $array);
		return $array;
	}
}

the assertion could even be

assertType('array{a: string, b: int, c: int}', $array);

When iterating over a constant array like ['b', 'c'] with a foreach loop,
all elements are guaranteed to be iterated. The fixed-point loop analysis
processes the body with the union type ('b'|'c'), which results in optional
keys since only one value is set per iteration.

This adds a post-loop unrolling refinement step that processes the body once
per element with specific types. This correctly determines that all keys are
definitely set after the loop completes, changing the result from
array{a: string, b?: int, c?: int} to array{a: string, b: int, c: int}.

The unrolling is limited to constant arrays with <= 8 elements, no optional
keys, no break statements, and no by-reference iteration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Collaborator Author

The changes have been pushed. Here's a summary of what was done:

Problem: The reviewer pointed out that after foreach (['b', 'c'] as $letter) { $array[$letter] = $i; }, the assertion should be array{a: string, b: int, c: int} (non-optional keys) rather than array{a: string, b?: int, c?: int} (optional keys), since the loop iterates over all elements.

Root cause: PHPStan's fixed-point loop analysis processes the body with the union type 'b'|'c' for the iteration variable. Since only one value is used per iteration, both keys are marked optional. The generalizeWith step during convergence preserves this optionality, making it impossible for the fixed-point approach alone to determine that all keys are definitely set.

Solution: Added a post-loop unrolling refinement step in NodeScopeResolver.php (after line 1384). When conditions are met:

  • Iterable is a constant array with all required keys and ≤ 8 elements
  • No break statements, no by-reference iteration
  • Loop is guaranteed to iterate at least once

The code processes the body once per element with specific (non-union) types. This correctly determines that after iterating over all elements, each key is definitely set. Only constant array variables with fewer optional keys in the unrolled result are updated in the final scope.

Test update: Updated bug-12665.php assertion from array{a: string, b?: int, c?: int} to array{a: string, b: int, c: int}.

Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert the last commit, we'll do this later in another PR.

@phpstan-bot
Copy link
Collaborator Author

Already retrieved this output as well. Everything is done - the revert commit has been pushed to the remote branch. Tests and static analysis both passed (the one test failure in RawErrorFormatterTest is pre-existing and unrelated).

Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… preservation

Add non-regression tests for:
- phpstan/phpstan#14080: array key type preserved through foreach loop
- phpstan/phpstan#13623: nested array with dynamic keys and ??= operator
- phpstan/phpstan#8774: constant string keys not overwritten by loop
- phpstan/phpstan#9907: array shape preserved with union key parameter
- phpstan/phpstan#11006: array shape preserved with nullable union key

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Collaborator Author

Done. Added 5 regression test files for the related issues identified by VincentLanglet:

  • bug-14080.php - Verifies array key type ('time') is preserved through a foreach loop that adds different keys
  • bug-13623.php - Verifies nested arrays with dynamic keys and ??= operator work correctly
  • bug-8774.php - Verifies constant string keys ('Reset') aren't overwritten by a loop adding different keys, preserving the exact string literal type
  • bug-9907.php - Verifies array shape is preserved when using a 'foo'|'bar' union key parameter
  • bug-11006.php - Verifies array shape is preserved when using a null|'size_uk'|'size_us' nullable union key

All tests pass, static analysis is clean, and the only test failure is the pre-existing RawErrorFormatterTest issue.

Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert the latest commit and add new regressions tests

  • For phpstan/phpstan#14080 we don't want an nsrt test but we want a rule test ensuring error like Binary operation "+=" between mixed and int<0, 3> results in an error. does not occurs anymore

  • For phpstan/phpstan#13623 we want a rule test checking Offset 'new_invoice' on array{balance_forward: 0, new_invoice: 0, payments: 0, balance: mixed} on left side of ??= always exists and is not nullable. does not occurs anymore

  • For phpstan/phpstan#8774 we want a rule test checking the Call to sprintf contains ... placeholders, ... value given. doesn't occur anymore

  • For phpstan/phpstan#9907, the NSRT is ok.

  • For phpstan/phpstan#11006 considers ProductParentPayloadDto->$SizeAttributeCode is not nullable and add add a rule test ensuring the error Parameter $values of class AkeneoUpdateProductDto constructor expects array{ean?: array<StringOrNullAttributeDto>, osa_sizes?: array<StringAttributeDto>, size_uk?: array<StringOrNullAttributeDto>, size_us?: array<StringOrNullAttributeDto>}, non-empty-array<'ean'|'osa_sizes'|'size_uk'|'size_us', array{StringAttributeDto}|array{StringOrNullAttributeDto}> given. does not occurs.

phpstan-bot and others added 2 commits March 14, 2026 10:41
… preservation

- Bug 14080: Rule test ensuring no binary operation error on += with preserved array shape
- Bug 13623: Rule test for ??= with nested dynamic-key arrays (documents remaining errors)
- Bug 8774: Rule test ensuring no sprintf placeholder mismatch with dynamic array keys
- Bug 9907: NSRT test for array shape preservation with union key parameter
- Bug 11006: Rule test ensuring no constructor parameter type mismatch with union key

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Collaborator Author

Already retrieved - all tests pass (only the pre-existing RawErrorFormatterTest failure). Everything has been pushed to the remote.

@VincentLanglet VincentLanglet self-assigned this Mar 14, 2026
@VincentLanglet VincentLanglet requested a review from staabm March 14, 2026 13:12
@phpstan-bot
Copy link
Collaborator Author

The branch is clean and up to date with the remote. The review is approved with no further changes needed. The PR already has all the requested regression tests and fixes in place.

Since the review is approved and there are no changes to make, there's nothing to address. The PR is ready to be merged.

@ondrejmirtes ondrejmirtes merged commit 61cdf12 into phpstan:2.1.x Mar 14, 2026
645 of 651 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants